-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[libc] Implement CMPLX related macros #156344
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
@llvm/pr-subscribers-libc Author: Connector Switch (c8ef) ChangesFull diff: https://github.com/llvm/llvm-project/pull/156344.diff 4 Files Affected:
diff --git a/libc/docs/headers/complex.rst b/libc/docs/headers/complex.rst
index 272cf00c883bc..7195f2695457e 100644
--- a/libc/docs/headers/complex.rst
+++ b/libc/docs/headers/complex.rst
@@ -10,7 +10,7 @@ Macros
+-----------+------------------+-----------------+------------------------+----------------------+------------------------+------------------------+----------------------------+
| <Func> | <Func_f> (float) | <Func> (double) | <Func_l> (long double) | <Func_f16> (float16) | <Func_f128> (float128) | C23 Definition Section | C23 Error Handling Section |
+===========+==================+=================+========================+======================+========================+========================+============================+
-| CMPLX | | | | | | 7.3.9.3 | N/A |
+| CMPLX | |check| | |check| | |check| | |check| | |check| | 7.3.9.3 | N/A |
+-----------+------------------+-----------------+------------------------+----------------------+------------------------+------------------------+----------------------------+
Functions
diff --git a/libc/include/llvm-libc-macros/CMakeLists.txt b/libc/include/llvm-libc-macros/CMakeLists.txt
index 7aa549ddc75d9..055e11f958e02 100644
--- a/libc/include/llvm-libc-macros/CMakeLists.txt
+++ b/libc/include/llvm-libc-macros/CMakeLists.txt
@@ -61,12 +61,6 @@ add_macro_header(
fcntl-macros.h
)
-add_macro_header(
- complex_macros
- HDR
- complex-macros.h
-)
-
add_macro_header(
features_macros
HDR
@@ -103,6 +97,14 @@ add_macro_header(
float16-macros.h
)
+add_macro_header(
+ complex_macros
+ HDR
+ complex-macros.h
+ DEPENDS
+ .float16_macros
+)
+
add_macro_header(
limits_macros
HDR
diff --git a/libc/include/llvm-libc-macros/complex-macros.h b/libc/include/llvm-libc-macros/complex-macros.h
index 427c68d289e0b..fdcc9c7831dd1 100644
--- a/libc/include/llvm-libc-macros/complex-macros.h
+++ b/libc/include/llvm-libc-macros/complex-macros.h
@@ -9,6 +9,8 @@
#ifndef __LLVM_LIBC_MACROS_COMPLEX_MACROS_H
#define __LLVM_LIBC_MACROS_COMPLEX_MACROS_H
+#include "float16-macros.h"
+
#ifndef __STDC_NO_COMPLEX__
#define __STDC_VERSION_COMPLEX_H__ 202311L
@@ -19,6 +21,18 @@
// TODO: Add imaginary macros once GCC or Clang support _Imaginary builtin-type.
+#define CMPLX(x, y) __builtin_complex((double)(x), (double)(y))
+#define CMPLXF(x, y) __builtin_complex((float)(x), (float)(y))
+#define CMPLXL(x, y) __builtin_complex((long double)(x), (long double)(y))
+
+#ifdef LIBC_TYPES_HAS_FLOAT16
+#define CMPLXF16(x, y) __builtin_complex((float16)(x), (float16)(y))
+#endif // LIBC_TYPES_HAS_FLOAT16
+
+#ifdef LIBC_TYPES_HAS_FLOAT128
+#define CMPLXF128(x, y) __builtin_complex((float128)(x), (float128)(y))
+#endif // LIBC_TYPES_HAS_FLOAT128
+
#endif
#endif // __LLVM_LIBC_MACROS_COMPLEX_MACROS_H
diff --git a/libc/test/include/complex_test.cpp b/libc/test/include/complex_test.cpp
index da833fb527381..869d2a935986e 100644
--- a/libc/test/include/complex_test.cpp
+++ b/libc/test/include/complex_test.cpp
@@ -17,3 +17,12 @@ TEST(LlvmLibcComplexTest, VersionMacro) {
TEST(LlvmLibcComplexTest, IMacro) { EXPECT_CFP_EQ(1.0fi, I); }
TEST(LlvmLibcComplexTest, _Complex_IMacro) { EXPECT_CFP_EQ(1.0fi, _Complex_I); }
+
+TEST(LlvmLibcComplexTest, CMPLXMacro) {
+ EXPECT_CFP_EQ(CMPLX(0, 1.0), I);
+ EXPECT_CFP_EQ(CMPLX(1.0, 0), 1.0);
+ EXPECT_CFP_EQ(CMPLXF(0, 1.0f), I);
+ EXPECT_CFP_EQ(CMPLXF(1.0f, 0), 1.0f);
+ EXPECT_CFP_EQ(CMPLXL(0, 1.0l), I);
+ EXPECT_CFP_EQ(CMPLXL(1.0l, 0), 1.0l);
+}
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assuming this is supported on both clang and gcc versions we use
This builtin was introduced in gcc 4.7 and clang 12. Safe to use I think. |
#define CMPLXL(x, y) __builtin_complex((long double)(x), (long double)(y)) | ||
|
||
#ifdef LIBC_TYPES_HAS_FLOAT16 | ||
#define CMPLXF16(x, y) __builtin_complex((float16)(x), (float16)(y)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have tests for float16 and float128 also?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using _Complex _Float16 appears to result in an error:
llvm-project/clang/lib/Sema/SemaChecking.cpp
Line 5554 in 45dec71
// We don't allow _Complex _Float16 nor _Complex __fp16 as type specifiers; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Float128 case added.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you create an issue for clang and add a TODO in the test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure why here _Complex _Float16
is invalid, gcc supports it: https://godbolt.org/z/5EYnY6f9z.
cc @zygoloid
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you create an issue for clang and add a TODO in the test?
Done.
✅ With the latest revision this PR passed the C/C++ code formatter. |
Fixed it 62447ef |
Thanks for the quick fix! I'll wait a day or two for it to be integrated into the clang-22 dev build. |
No description provided.